Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update K8s dashboard chart version #68

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

gabor-boros
Copy link
Contributor

This PR bumps the K8s chart version to the latest available, which provides TLS configuration options. Hence, we are able to set SSL certificates for service, and expose it.

@gabor-boros gabor-boros added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Mar 27, 2024
@gabor-boros gabor-boros self-assigned this Mar 27, 2024
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 27, 2024
@openedx-webhooks
Copy link

Thanks for the pull request, @gabor-boros! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

1 similar comment
@openedx-webhooks
Copy link

Thanks for the pull request, @gabor-boros! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@gabor-boros gabor-boros force-pushed the gabor/update-k8s-dashboard-chart branch from b83b163 to 93ec805 Compare April 5, 2024 05:08
@gabor-boros
Copy link
Contributor Author

@felipemontoya this has been rebased. Now it can be reviewed anytime.

@gabor-boros
Copy link
Contributor Author

@felipemontoya a gentle reminder here 😇

@felipemontoya
Copy link
Member

Hi @gabor-boros, sorry for taking so long.

This PR makes absolute sense. It might be even late as there is now a 7.3.1 (completely fault of the late reviewer).

I reviewed the notes from the package and it says:

Update from 7.x.x-alphaX to 7.x.x

Due to further architecture changes do a clean installation of Kubernetes Dashboard when upgrading from alpha chart version. Default dependency on both ingress-nginx-controller and cert-manager have been removed in favor of using a single-container, DBless kong installation as a gateway that connects all our containers and exposes the UI. Users can then use any ingress controller or proxy in front of kong gateway.

Did you run into this when you did the change locally? Are we moderately confident that most installers won't have an issue?

Maybe we should start the practice of leaving changenotes so there is a summary of operations made available for users. I don't think we should block the PR though. I will leave an approval in a moment.

@gabor-boros gabor-boros force-pushed the gabor/update-k8s-dashboard-chart branch from 93ec805 to 9c6f9da Compare April 15, 2024 10:38
@gabor-boros gabor-boros force-pushed the gabor/update-k8s-dashboard-chart branch from 9c6f9da to 0e6dc75 Compare April 15, 2024 10:42
@gabor-boros
Copy link
Contributor Author

It might be even late as there is now a 7.3.1 (completely fault of the late reviewer).

Nice! I just updated the dependency before merging the PR to ensure the latest chart is used.

Did you run into this when you did the change locally? Are we moderately confident that most installers won't have an issue?

Yes I did. However, this only happens when someone wants to expose the dashboard using an ingress controller (and don't want to do port-forwarding). Since it wasn't available before to do anything else but port-forwarding, I'm quite confident that most people won't have issues. Also, I'm not even sure that many people using the Kubernetes dashboard, as it must be explicitly enabled.

Maybe we should start the practice of leaving changenotes so there is a summary of operations made available for users.

That's a great idea! How about discussing that on the next meeting? I'd love to have some change notes.

@gabor-boros gabor-boros merged commit fc4a25e into main Apr 15, 2024
2 checks passed
@gabor-boros gabor-boros deleted the gabor/update-k8s-dashboard-chart branch April 15, 2024 10:52
@openedx-webhooks
Copy link

@gabor-boros 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants